Skip to content

Conversation

@philnik777
Copy link
Contributor

No description provided.

@github-actions
Copy link

github-actions bot commented Nov 27, 2025

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff origin/main HEAD --extensions ,cpp -- libcxx/include/string libcxx/test/benchmarks/containers/string.bench.cpp --diff_from_common_commit

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.
diff --git a/libcxx/test/benchmarks/containers/string.bench.cpp b/libcxx/test/benchmarks/containers/string.bench.cpp
index 68249ad89..8fb1d1a21 100644
--- a/libcxx/test/benchmarks/containers/string.bench.cpp
+++ b/libcxx/test/benchmarks/containers/string.bench.cpp
@@ -128,7 +128,7 @@ BENCHMARK(BM_StringResizeAndOverwrite);
 template <class Container>
 static void BM_StringAppend(benchmark::State& state) {
   std::string orig_str = "This is some data so the string isn't empty";
-  std::string str = orig_str;
+  std::string str      = orig_str;
 
   const char to_append_str[] = "This is a long string to make sure append does some work";
   Container to_append;

@philnik777 philnik777 force-pushed the optimize_string_append_range branch 2 times, most recently from e850aa4 to f4596cb Compare November 28, 2025 13:15
@philnik777 philnik777 force-pushed the optimize_string_append_range branch from f4596cb to 1ac38dc Compare December 2, 2025 14:55
@ldionne ldionne marked this pull request as ready for review December 2, 2025 16:11
@ldionne ldionne requested a review from a team as a code owner December 2, 2025 16:11
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 2, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 2, 2025

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/169794.diff

2 Files Affected:

  • (modified) libcxx/include/string (+40-13)
  • (modified) libcxx/test/benchmarks/containers/string.bench.cpp (+23)
diff --git a/libcxx/include/string b/libcxx/include/string
index 2b3ba6d2d9b62..c0454ba2cb9f8 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1440,24 +1440,51 @@ public:
   template <class _ForwardIterator, __enable_if_t<__has_forward_iterator_category<_ForwardIterator>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string&
   append(_ForwardIterator __first, _ForwardIterator __last) {
-    size_type __sz  = size();
-    size_type __cap = capacity();
-    size_type __n   = static_cast<size_type>(std::distance(__first, __last));
+    const size_type __size = size();
+    const size_type __cap  = capacity();
+    const size_type __n    = static_cast<size_type>(std::distance(__first, __last));
     if (__n == 0)
       return *this;
 
-    if (__string_is_trivial_iterator_v<_ForwardIterator> && !__addr_in_range(*__first)) {
-      if (__cap - __sz < __n)
-        __grow_by_without_replace(__cap, __sz + __n - __cap, __sz, __sz, 0);
-      __annotate_increase(__n);
-      auto __end = __copy_non_overlapping_range(__first, __last, std::__to_address(__get_pointer() + __sz));
-      traits_type::assign(*__end, value_type());
-      __set_size(__sz + __n);
-      return *this;
+    __annotate_delete();
+    auto __asan_guard = std::__make_scope_guard(__annotate_new_size(*this));
+
+    if (__n > __cap - __size) {
+      __long __buffer  = __allocate_long_buffer(__alloc_, __get_amortized_growth_capacity(__size + __n));
+      __buffer.__size_ = __size + __n;
+      auto __guard     = std::__make_exception_guard([&] {
+        __alloc_traits::deallocate(__alloc_, __buffer.__data_, __buffer.__cap_ * __endian_factor);
+      });
+      auto __end       = __copy_non_overlapping_range(__first, __last, std::__to_address(__buffer.__data_) + __size);
+      traits_type::assign(*__end, _CharT());
+      traits_type::copy(std::__to_address(__buffer.__data_), data(), __size);
+      __guard.__complete();
+      __reset_internal_buffer(__buffer);
     } else {
-      const basic_string __temp(__first, __last, __alloc_);
-      return append(__temp.data(), __temp.size());
+      _CharT* const __ptr = std::__to_address(__get_pointer());
+#  ifndef _LIBCPP_CXX03_LANG
+      if constexpr (__libcpp_is_contiguous_iterator<_ForwardIterator>::value &&
+                    is_same<value_type, __remove_cvref_t<decltype(*__first)>>::value) {
+        traits_type::move(__ptr + __size, std::__to_address(__first), __last - __first);
+      } else if constexpr (__has_bidirectional_iterator_category<_ForwardIterator>::value) {
+        auto __dest = __ptr + __size + __n;
+        while (__first != __last) {
+          --__last;
+          --__dest;
+          traits_type::assign(*__dest, *__last);
+        }
+      } else
+#  endif
+      {
+        _CharT __first_val = *__first;
+        ++__first;
+        __copy_non_overlapping_range(__first, __last, __ptr + __size + 1);
+        traits_type::assign(__ptr[__size], __first_val);
+      }
+      traits_type::assign(__ptr[__size + __n], _CharT());
+      __set_size(__size + __n);
     }
+    return *this;
   }
 
 #  if _LIBCPP_STD_VER >= 23
diff --git a/libcxx/test/benchmarks/containers/string.bench.cpp b/libcxx/test/benchmarks/containers/string.bench.cpp
index 98216d22d0144..68249ad8963ed 100644
--- a/libcxx/test/benchmarks/containers/string.bench.cpp
+++ b/libcxx/test/benchmarks/containers/string.bench.cpp
@@ -11,7 +11,10 @@
 #include <algorithm>
 #include <cstdint>
 #include <cstdlib>
+#include <deque>
+#include <iterator>
 #include <new>
+#include <list>
 #include <vector>
 
 #include "../CartesianBenchmarks.h"
@@ -122,6 +125,26 @@ static void BM_StringResizeAndOverwrite(benchmark::State& state) {
 }
 BENCHMARK(BM_StringResizeAndOverwrite);
 
+template <class Container>
+static void BM_StringAppend(benchmark::State& state) {
+  std::string orig_str = "This is some data so the string isn't empty";
+  std::string str = orig_str;
+
+  const char to_append_str[] = "This is a long string to make sure append does some work";
+  Container to_append;
+  std::copy(std::begin(to_append_str), std::end(to_append_str), std::back_inserter(to_append));
+
+  for (auto _ : state) {
+    benchmark::DoNotOptimize(str);
+    str.append(to_append.begin(), to_append.end());
+    benchmark::DoNotOptimize(str);
+    str.resize(orig_str.size());
+  }
+}
+BENCHMARK(BM_StringAppend<std::vector<char>>);
+BENCHMARK(BM_StringAppend<std::deque<char>>);
+BENCHMARK(BM_StringAppend<std::list<char>>);
+
 enum class Length { Empty, Small, Large, Huge };
 struct AllLengths : EnumValuesAsTuple<AllLengths, Length, 4> {
   static constexpr const char* Names[] = {"Empty", "Small", "Large", "Huge"};

@ldionne
Copy link
Member

ldionne commented Dec 2, 2025

/libcxx-bot benchmark libcxx/test/benchmarks/containers/string.bench.cpp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants